Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add decision record for the removal of the init() method #33

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

romaricpascal
Copy link
Member

No description provided.

decision-records/010-remove-init-method.md Show resolved Hide resolved
decision-records/010-remove-init-method.md Outdated Show resolved Hide resolved
decision-records/010-remove-init-method.md Outdated Show resolved Hide resolved
decision-records/010-remove-init-method.md Outdated Show resolved Hide resolved
decision-records/010-remove-init-method.md Outdated Show resolved Hide resolved
decision-records/010-remove-init-method.md Outdated Show resolved Hide resolved
decision-records/010-remove-init-method.md Outdated Show resolved Hide resolved
decision-records/010-remove-init-method.md Outdated Show resolved Hide resolved
decision-records/010-remove-init-method.md Outdated Show resolved Hide resolved

Component initialisation will be reduced to `new ComponentClass()` instead of `new ComponentClass().init()`, so users will need to update their code to reflect this.

This will only affect users that were taking responsibility for instantiating components on specific elements themselves. Users who rely on `initAll()` will not have any changes to make as we'll have updated that function's implementation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, felt like the words were hiding the meaning:

Suggested change
This will only affect users that were taking responsibility for instantiating components on specific elements themselves. Users who rely on `initAll()` will not have any changes to make as we'll have updated that function's implementation.
The `initAll()` method will not be affected.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind if I mix both approaches, to clarify which users will need to update?

Suggested change
This will only affect users that were taking responsibility for instantiating components on specific elements themselves. Users who rely on `initAll()` will not have any changes to make as we'll have updated that function's implementation.
The `initAll()` method will not be affected. Only users that were instantiating components on specific elements themselves will need to update their code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we've already got it above, leave it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't hurt to have it there as well if someone gets linked directly to this section (they might not scroll back up).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@colinrotherham Updated that bit, which I'd missed 😊

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good in isolation, but repeats text from the paragraph above.

How about my new suggestion in #33 (comment) and #33 (review)?

@36degrees 36degrees force-pushed the throw-errors-js-constructors branch from 4608371 to ab64f7a Compare July 28, 2023 08:36
@36degrees 36degrees force-pushed the throw-errors-js-constructors branch from ab64f7a to 10a04d4 Compare July 28, 2023 08:53
Base automatically changed from throw-errors-js-constructors to main July 28, 2023 09:06
Co-authored-by: Colin Rotherham <work@colinr.com>
@romaricpascal romaricpascal merged commit e60350d into main Jul 31, 2023
@romaricpascal romaricpascal deleted the remove-init-method branch July 31, 2023 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants